Skip to content

Deprecate cudf::grouped_time_range_rolling_window#17589

Merged
rapids-bot[bot] merged 4 commits intorapidsai:branch-25.02from
wence-:wence/fea/rolling-deprecate
Jan 20, 2025
Merged

Deprecate cudf::grouped_time_range_rolling_window#17589
rapids-bot[bot] merged 4 commits intorapidsai:branch-25.02from
wence-:wence/fea/rolling-deprecate

Conversation

@wence-
Copy link
Contributor

@wence- wence- commented Dec 13, 2024

Description

Since the range-based rolling window APIs were extended to support any orderable column (rather than just time-based windows), this function has been a thin forwarding wrapper to
cudf::grouped_range_rolling_window. It could have been deprecated at the time, but was not. It seems there are no usages outside of the libcudf test suite.

Partially addresses #13050.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@wence- wence- requested a review from a team as a code owner December 13, 2024 11:08
@wence- wence- requested review from lamarrr and mythrocks December 13, 2024 11:08
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Dec 13, 2024
@wence- wence- added improvement Improvement / enhancement to an existing function breaking Breaking change labels Dec 13, 2024
@mythrocks
Copy link
Contributor

Thank you, @wence-. I've been dragging my feet on this one.

I have checked. It appears that I've already removed references to the time-range version of these calls from the Java API, and switched it to numeric ranges.

spark-rapids and spark-rapids-jni should not be affected by this: All references should be through JNI, which has already moved.

This is looking good to go.

@mythrocks
Copy link
Contributor

mythrocks commented Dec 13, 2024

The failures seem to be transient and unrelated to the change.

Cloning into 'rapids_logger-src'...
fatal: unable to read tree (14bb233d2420f7187a690f0bb528ec0420c70d48)
CMake Error at rapids_logger-subbuild/rapids_logger-populate-prefix/tmp/rapids_logger-populate-gitclone.cmake:61 (message):
  Failed to checkout tag: '14bb233d2420f7187a690f0bb528ec0420c70d48'


FAILED: rapids_logger-populate-prefix/src/rapids_logger-populate-stamp/rapids_logger-populate-download /__w/cudf/cudf/cpp/build/_deps/rapids_logger-subbuild/rapids_logger-populate-prefix/src/rapids_logger-populate-stamp/rapids_logger-populate-download 
cd /__w/cudf/cudf/cpp/build/_deps && /opt/conda/envs/clang_tidy/bin/cmake -DCMAKE_MESSAGE_LOG_LEVEL=VERBOSE -P /__w/cudf/cudf/cpp/build/_deps/rapids_logger-subbuild/rapids_logger-populate-prefix/tmp/rapids_logger-populate-gitclone.cmake && /opt/conda/envs/clang_tidy/bin/cmake -E touch /__w/cudf/cudf/cpp/build/_deps/rapids_logger-subbuild/rapids_logger-populate-prefix/src/rapids_logger-populate-stamp/rapids_logger-populate-download
ninja: build stopped: subcommand failed.

I've restarted the failing CI jobs.

@wence- wence- force-pushed the wence/fea/rolling-deprecate branch 2 times, most recently from 6e6b5b4 to 3bb751d Compare December 17, 2024 12:01
Since the range-based rolling window APIs were extended to support any
orderable column (rather than just time-based windows), this function
has been a thin forwarding wrapper to
cudf::grouped_range_rolling_window. It could have been deprecated at
the time, but was not. It seems there are no usages outside of the
libcudf test suite.

Partially addresses rapidsai#13050.
@wence- wence- force-pushed the wence/fea/rolling-deprecate branch from 3bb751d to f5366e5 Compare December 17, 2024 15:30
Copy link
Contributor

@lamarrr lamarrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. thanks for working on this!

@wence-
Copy link
Contributor Author

wence- commented Jan 20, 2025

/merge

@rapids-bot rapids-bot bot merged commit 9cd5253 into rapidsai:branch-25.02 Jan 20, 2025
114 checks passed
@wence- wence- deleted the wence/fea/rolling-deprecate branch January 21, 2025 13:00
@GregoryKimball GregoryKimball removed this from libcudf Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants